Size Change: +4.86 kB (0%)

Total Size: 1.41 MB

_{compressed-size-action}

Flaky tests detected in 82a7024.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5074114556
📝 Reported issues:

• [Flaky Test] No Behaviors should be the default as defined in the core theme.json #50923 in /test/e2e/specs/editor/various/behaviors.spec.js

Hey @glendaviesnz I've just found this PR and it seems there's some disagreements/confusion about what needs to be done, that we need to clear out.

Separately I've started this PR #50369 as a starting point to address #50378 which also includes a "view post/view page" links. I don't know if we're supposed to have both of these? Anyway, just wanted to link to these and loop in designers to get more clarity @jameskoster @jasmussen @SaxonF

Riad there's a bit more context in #50405 (comment) for this, cc: @richtabor

Separately I've started this PR #50369 as a starting point to address #50378 which also includes a "view post/view page" links. I don't know if we're supposed to have both of these?

The issue is to add a view site link outside of the Edit View, so you don't have to navigate first into a template/page to then view the site.

The main question for me is whether the link should always open the homepage, or be contextual and open whatever is visible in the frame (assuming it's compatible).

If it's a generic "open the homepage" link, then the placement seems good. Though I'd echo Joen's comment about making use of the site title rather than adding another icon button.

If the link is to be contextually related to the frame then I would question the placement a bit, and suggest exploring something more connected to the frame.

The main question for me is whether the link should always open the homepage, or be contextual and open whatever is visible in the frame (assuming it's compatible).

I'm thinking homepage.

Okay.

If we made use of the site title, we could still use the icon attached in a smaller variant:

We use a similar pattern elsewhere:

Or maybe a simple text-decoration: underline; would be adequate?

I would think the underline is sufficient, but either way can we surface the icon on hover only?

Underline works for me, and avoids the inconsistent icon sizing which looks a bit awkward.

Can even combine with tooltip if that's not enough. Lots of options!

I have updated it to an underlined link with tooltip:

Tooltip, no underline, is my vote 😅

I wouldn't mind the underline on hover, but agree the resting state should not have an underline. Thanks for keeping at this!

I wouldn't mind the underline on hover, but agree the resting state should not have an underline.

Done 😃

It was mentioned yesterday that using the site title can create confusion whereby you mistakenly interpret the icon + title to be a single element, which can result in mis-clicks.

An alternative option which avoids adding noise would be to only expose the externalink when you hover the site hub:

I suppose it would need to be permanently visible on mobile, but that seems to be a reasonable trade-off. What do y'all think?

An alternative option which avoids adding noise would be to only expose the externalink when you hover the site hub

I don't mind this.

An alternative option which avoids adding noise would be to only expose the externalink when you hover the site hub

I don't mind this.

@jasmussen are you also happy with this approach?

Sure let's go with that.

I still feel like we kind of urgently need a better external link icon, it's huge. Here's a sketch that updates the View menu as well:

That's not blocking for now, just a design task.

I have switched it to the hover only icon:

@jasmussen 18px x 18px seems to balance reasonably well against the command center search icon, but let me know what sort of sizing you think is best.

I have switched to button and also set the icon to display if the hub content area is focused, but I had trouble getting that to focus using the keyboard.

A couple of details (apologies, you must be tired of this one):

• The button disappears when you hover the search button which feels a bit unexpected. Ideally it's visible when the cursor is inside edit-site-site-hub.
• The space between external / search buttons is 8px which is a touch large. If we can reduce that to 4px it looks a little neater.

Aside from that and any outstanding a11y issues we should be good to merge.

@ciampo I know you have much on your table. I wonder, should we create a GH board to track all these small component issues that aren't necessarily blocking, but would be good to come back to at some point?

Hey @jasmussen , that sounds good to me. We won't necessarily have the capacity to work on these issues firsthand, but we're always happy to help with reviews and advice (and with some authoring when necessary like we did on the editor frame resizing PR)

@jameskoster I have fixed the hover issue and reduced the gap.

Thanks Glen! Pending a11y sign-off this is looking good to go.

Accessibility-wise:

• Keyboard navigable ✅
• Button (actually an <a>) semanticallly correct and labeled ✅
• We should probably append a warning that the link opens in a new tab. Does the Button component allow different content in the tooltip and the aria label? So like label="View site" (for tooltip) and aria-label="View site (opens in a new tab)" (for SRs only).

• Does the Button component allow different content in the tooltip and the aria label? So like label="View site" (for tooltip) and aria-label="View site (opens in a new tab)" (for SRs only).

Yes, that seems to work, have added that.

I am going to go ahead and merge as it should be easy to follow up on if any other accessibility issues are raised.

Just noticed that this PR broke the layout of the Site hub when tabbing with the keyboard. Created #51257 and suggested a possible fix there.

A couple additional accessibility follow-ups:

1. The focus state for this control doesn't match neighboring controls:

2. The tooltip text doesn't notify the user the link will open in a new tab., it's only available in the aria-label.

3. What is the reasoning behind having this link hidden by default? This significantly reduces discoverability, and I'm not clear that it provides any advantages.

@joedolson I have added a PR to fix the focus and label issues. @jameskoster would be the person to answer the question about why it is only visible on mouseover/focus.

Iirc, as it is not 100% clear how the 'site hub' will evolve as we explore the broader admin design there was motivation to keep it as lightweight as possible for now. Nothing is set in stone, and it may be something we revisit later.

We may have different definitions of lightweight; I see the hidden link as a more complex design with a higher cognitive and discovery load. However, I do appreciate that until there's some kind of admin and management component in place, there isn't really a well-defined context for this. But not a fan of hidden controls.

But not a fan of hidden controls.

I'd totally second this, and I'd like to add that in the last 6 years one of the most common accessibility concerns that was very often reported is specifically about UI controls that continuously appear and disappear. That's an accessibility anti-pattern and I'd greatly appreciate all designers to be more aware of that. Thank you.